Skip to content

LDAP adoption documentation #980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 15, 2025
Merged

LDAP adoption documentation #980

merged 1 commit into from
Aug 15, 2025

Conversation

@xek
Copy link
Contributor

@xek xek commented Jun 25, 2025

No description provided.

@xek xek mentioned this pull request Jun 25, 2025
Open
@xek xek force-pushed the ldap-adoption-docs branch from 1ee7db0 to 9ca3ddd Compare June 25, 2025 12:28
@xek xek requested a review from klgill June 25, 2025 12:29
@xek
Copy link
Contributor Author

xek commented Jun 25, 2025

note: this was already reviewed by @klgill in #970

@xek xek requested a review from jistr June 25, 2025 12:52
@xek xek force-pushed the ldap-adoption-docs branch from 9ca3ddd to 50b0432 Compare June 25, 2025 13:11
klgill
klgill requested changes Jun 25, 2025
View reviewed changes
Copy link
Contributor

@klgill klgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xek I just have a few minor formatting changes that need to be implemented. After you commit the changes, I can look at the preview again and make sure it looks right.
Nice work!

@xek xek force-pushed the ldap-adoption-docs branch 2 times, most recently from 5971858 to 77b1fe4 Compare June 25, 2025 15:28
@xek
Copy link
Contributor Author

xek commented Jun 25, 2025

@klgill Thanks for the quick reviews! I applied the changes.

@xek xek force-pushed the ldap-adoption-docs branch from ad67372 to aeb7df0 Compare June 26, 2025 10:31
@xek xek requested a review from klgill July 1, 2025 12:35
klgill
klgill approved these changes Jul 1, 2025
View reviewed changes
Copy link
Contributor

@klgill klgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ciecierski
ciecierski requested changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@ciecierski ciecierski left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few things to fix:

  • verification section missing
  • drop namespace from commands
  • formatting of oc patch command line
  • one inline fix

Also tests are separate pull-request. #970 I'm not sure if we agreed that it is fine to separate doc change and test change or not.

@xek xek force-pushed the ldap-adoption-docs branch from aeb7df0 to fa2ae3b Compare July 10, 2025 09:31
@xek
Copy link
Contributor Author

xek commented Jul 10, 2025

@ciecierski Thanks for the review! I added all of the changes including the verification steps.

@xek xek requested a review from ciecierski July 10, 2025 09:58
klgill
klgill requested changes Jul 10, 2025
View reviewed changes
@xek @klgill
LDAP adoption documentation
ad3b2c8 
Co-authored-by: Katie Gilligan <[email protected]>
@xek xek force-pushed the ldap-adoption-docs branch from c56db2b to ad3b2c8 Compare July 14, 2025 10:59
@softwarefactory-project-zuul
Copy link

softwarefactory-project-zuul bot commented Jul 14, 2025

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a827c179f2604d93a59bf94bee3e7e88

✔️ noop SUCCESS in 0s
❌ adoption-docs-preview FAILURE in 1m 19s

@xek
Copy link
Contributor Author

xek commented Jul 14, 2025

@klgill Thanks!

@xek xek requested a review from klgill July 14, 2025 10:59
klgill
klgill approved these changes Jul 14, 2025
View reviewed changes
Copy link
Contributor

@klgill klgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jul 14, 2025
@ciecierski
Copy link
Contributor

ciecierski commented Jul 14, 2025

@jistr I let it up to you if tests in separate pull-request are fine. #970

@github-actions
Copy link

github-actions bot commented Jul 30, 2025

This PR is stale because it has been for over 15 days with no activity.
Remove stale label or comment or this PR will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jul 30, 2025
@github-actions github-actions bot closed this Aug 6, 2025
@jistr jistr reopened this Aug 11, 2025
@jistr jistr removed the Stale label Aug 11, 2025
@jistr
Copy link
Contributor

jistr commented Aug 15, 2025 •
edited
Loading

The docs LGTM as well and i think this is a standalone feature that is not on the critical path for testing, so we can merge the docs separately from having the tests implemented.
/lgtm
/approve

@openshift-ci
Copy link

openshift-ci bot commented Aug 15, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jistr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit a05cdd1 into openstack-k8s-operators:main Aug 15, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@klgill klgill klgill approved these changes

@jistr jistr Awaiting requested review from jistr

@ciecierski ciecierski Awaiting requested review from ciecierski

Assignees

@jistr jistr

@klgill klgill

Labels

approved lgtm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xek @ciecierski @jistr @klgill